Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support canvas.getContext("2d", {alpha: boolean, pixelFormat: string}) #935

Merged
merged 4 commits into from
Jul 15, 2017

Conversation

zbjornson
Copy link
Collaborator

@zbjornson zbjornson commented Jul 2, 2017

This is an amped-up version of most of #678. ("Most" because it doesn't add the flush function or allow passing a buffer as that PR adds. "Amped-up" because all of the APIs work AFAICT, with the caveats noted in the docs.) My priority was to support indexed PNG encoding, but a lot of stuff came with it. I'm happy to split the 1st/2nd commits from the 3rd and 4th. Indexed PNG encoding is maybe out of scope and is something that could be extracted into a separate C++ addon, but the first two support the standard.

Docs for the major addition: https://github.com/zbjornson/node-canvas/tree/formats#image-pixel-formats-experimental

  • Adds support for the standard alpha attribute: canvas.getContext("2d", {alpha: false}). In the browser tests, node-canvas matches Firefox as well as ever, and Chrome seems to have a few bugs in which canvases randomly do get an alpha channel.

  • Adds support for the proposed pixelFormat attribute: canvas.getContext("2d", {pixelFormat: <format>}). The color space proposal is still fairly early, and in particular the format names look like they'll change (see the open issues), but adding aliases for other formats for backwards compatibility in the future will be trivial. Even if that proposal doesn't move forward, this is nice functionality to have and a clean API (i.e. it adds options to the standard contextOptions object instead of positional args to the Canvas constructor).

    • RGBA32, RGB24 and A8 have full support: getImageData/putImageData/createImageData all work.
    • RGB16_565 also has full support, but note: The ImageData.data property is a Uint16Array instead of a Uint8ClampedArray because it doesn't make sense as a uint8. This follows the linked proposal AFAICT -- ImageData has multiple storage types there, but it's not specified how bit widths like 5-6-5 map to integral types. This commit is easy to drop if there's any disagreement.
    • A1 and RGB30 have partial support: getImageData/putImageData/createImageData don't work because I wasn't sure how the data should be represented. Cairo packs pixels together but also has a padded stride (e.g. an A1 canvas with width 10 has a stride of 4 bytes). We could either pack all pixels together or keep them padded. This is a ton of bit manipulation given that these APIs can work on regions of a canvas. 😓
  • Adds support (non-standard) for indexed PNG encoding. 🎉 See the two new files in examples/. That particular image fits losslessly into 474 bytes when indexed (left), vs 1.92 kB when RGBA32 (right).

    indexed rgb
    Note that it's sorta hard to specify an exact palette index using the 0-1 value in rgba() syntax. Adding #rgba and #rrggbbaa support will allow integers to be provided.

    Right now only pngStream supports this, but if this PR lands, then toBuffer and maybe toDataURL should support it also. For the former, I'd propose moving all of the toBuffer options into an object since the current API is awkward (see Move PNG_* constants to constructor, clean up toBuffer API #934).


No breaking API changes.
Fixes #332

@zbjornson zbjornson force-pushed the formats branch 3 times, most recently from 35ba8fe to 465ea4c Compare July 2, 2017 22:40
@LinusU
Copy link
Collaborator

LinusU commented Jul 9, 2017

This looks amazing, great work! 🎉

I think I introduced some conflict, sorry about that, would you mind fixing them? 😀

@zbjornson zbjornson force-pushed the formats branch 2 times, most recently from f0d286d to 6b83f78 Compare July 11, 2017 02:38
@LinusU
Copy link
Collaborator

LinusU commented Jul 15, 2017

Awesome work as always!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants